perf: replace AccAddress.String() mutex+LRU with sync.Map#2902
Draft
perf: replace AccAddress.String() mutex+LRU with sync.Map#2902
Conversation
AccAddress.String() uses a global sync.Mutex to protect a simplelru.LRU cache. Under OCC with many goroutines, this serializes all bech32 lookups even on cache hits (LRU.Get modifies ordering, requiring a write lock). The mutex profile shows 26.79s (6.66%) of contention on this path, primarily from bank keeper event emission (AddCoins, Transfer, BuyGas). Replace with sync.Map which provides lock-free reads for cache hits. The tradeoff is losing LRU eviction, but practical address cardinality is bounded by on-chain accounts (similar order as the old 60k cap). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2902 +/- ##
==========================================
+ Coverage 57.21% 57.24% +0.02%
==========================================
Files 2093 2093
Lines 171755 171732 -23
==========================================
+ Hits 98276 98312 +36
+ Misses 64709 64667 -42
+ Partials 8770 8753 -17
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Slow path: compute bech32 and store with a stable key copy. | ||
| bech32Addr, err := bech32.ConvertAndEncode(GetConfig().GetBech32AccountAddrPrefix(), aa) | ||
| if err != nil { | ||
| panic(err) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
pdrobnjak
added a commit
that referenced
this pull request
Feb 17, 2026
Cosmos-level events (coin_spent, coin_received, transfer, etc.) emitted during giga executor tx execution are never consumed: the ExecTxResult is built without populating the Events field, and events are not consensus-critical (stripped from deterministicExecTxResult). EVM logs (Solidity events) flow through a completely separate path (tempState.logs -> receipts + MsgEVMTransactionResponse) and are unaffected. When GIGA_SUPPRESS_COSMOS_EVENTS=true: - EventManager.EmitEvent/EmitEvents return immediately (no mutex, no append, no bech32 encoding in event attributes) - Snapshot() creates suppressed EventManagers - Finalize() skips the flushEvents() consolidation loop This eliminates ~55s of mutex contention per 30s profile window: - AccAddress.String() bech32 cache mutex: ~27s - EventManager.EmitEvents RWMutex: ~28s Also updates benchmark/analysis/ with findings from the AccAddress.String sync.Map optimization (PR #2902) and this event suppression discovery. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
We truly appreciate your contribution and the time you’ve invested in this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sync.Mutex+simplelru.LRUinAccAddress.String()withsync.Mapfor lock-free readsContext
AccAddress.String()converts raw address bytes to bech32 encoding. The existing implementation uses a globalsync.Mutexto protect asimplelru.LRUcache. Even cache hits require a write lock becauseLRU.Get()modifies the recency ordering.Under OCC with many goroutines executing EVM transactions concurrently, this global mutex serializes all bech32 lookups across all worker goroutines, showing up as 26.79s (6.66%) in the mutex contention profile.
Change
Replace with
sync.Mapwhich provides:The tradeoff is losing LRU eviction, but practical address cardinality is bounded by on-chain accounts (similar order as the old 60k entry cap), so memory growth is bounded.
Benchmark Results
Mutex contention diff (focused on
AccAddress.String): -41.55s eliminatedTest plan
go test ./sei-cosmos/types/... -run 'Addr|Bech32|Address'— all passgofmt -s -l— clean🤖 Generated with Claude Code